fix(scripts): scale token amounts without a floating-point intermediary#194
Open
Nexory wants to merge 1 commit into
Open
fix(scripts): scale token amounts without a floating-point intermediary#194Nexory wants to merge 1 commit into
Nexory wants to merge 1 commit into
Conversation
The bridge and mint CLI handlers scaled the user-entered amount with `BigInt(Math.floor(amount * 10 ** decimals))`, where `amount` is a `parseFloat`-ed number. Evaluating `amount * 10 ** decimals` in IEEE-754 double precision loses accuracy: `1.005 * 1e9` is `1004999999.9999999`, so `Math.floor` produces `1_004_999_999` lamports instead of the intended `1_005_000_000`. `1.000001` at 6 decimals drops the trailing unit (`1_000_000` instead of `1_000_001`), and 18-decimal (wei) amounts exceed `Number.MAX_SAFE_INTEGER` and lose arbitrary trailing precision. Add a small `parseTokenAmount(value, decimals)` helper backed by viem's `parseUnits`, which parses the decimal string directly with no float intermediary, and use it in all six call sites: - bridge-sol, bridge-sol-with-bc, bridge-spl, bridge-wrapped-token (9 or mint-decimals) - bridge-call (18, ETH -> wei) - spl/mint (mint decimals) The amount/value Zod fields now keep the validated string instead of transforming to a float, so the raw decimal input reaches parseUnits intact. Logging is unchanged (the field is still a string). Verified locally: - bun test src/internal/amount.test.ts : 3 pass (documents the legacy float loss and pins the corrected output) - tsc --noEmit : 0 errors across the scripts package
Collaborator
🟡 Heimdall Review Status
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The Solana bridge and SPL mint CLI handlers scale the user-entered amount with a floating-point intermediary:
args.amountis aparseFloat-ed number (from the Zod.transform((val) => parseFloat(val))), andamount * 10 ** decimalsis evaluated in IEEE-754 double precision. That loses accuracy for ordinary decimal inputs.Why it is wrong
1.005 * 1e9evaluates to1004999999.9999999, soMath.floorrounds it down. The on-chain amount ends up smaller than what the user typed. The magnitude is small for 9/6-decimal tokens (one smallest-unit), but for 18-decimal (wei) values any amount past ~15-16 significant digits loses precision because the product exceedsNumber.MAX_SAFE_INTEGER.Fix
Add a small
parseTokenAmount(value, decimals)helper backed by viem'sparseUnits(already a dependency), which parses the decimal string directly with no float intermediary, and use it at all six call sites:bridge-sol,bridge-sol-with-bc,bridge-spl,bridge-wrapped-tokenbridge-call(ETH -> wei)spl/mintThe
amount/valueZod fields now keep the validated string instead of transforming to a float, so the raw decimal input reachesparseUnitsintact. Logging is unchanged (the field is still a string).parseUnitstruncates over-precision inputs the same way the previousMath.floordid, so no input that worked before changes behavior.Verification (local)
The test documents the legacy float loss (
1.005 @ 9 -> 1_004_999_999) and pins the corrected output (-> 1_005_000_000).Notes
scriptspackage (the workflows cover the Forge and Solana programs), so the verification above was run locally. Happy to wire abun test/tscstep into CI forscriptsin a follow-up if useful.